feat(integrations): add integrations settings by chickenn00dle · Pull Request #4541 · Automattic/newspack-plugin

chickenn00dle · 2026-03-04T16:56:54Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes https://linear.app/a8c/issue/NPPD-1325/framework-allow-integrations-to-declare-settings-fields-plus-prototype

Adds a settings framework for reader activation integrations. Integrations can now declare settings fields (text, checkbox, select, number, textarea, password) that are stored as WordPress options and managed via a new Audience Integrations admin page.

This PR also adds a prototype UI that mostly copies the UI for sync in Audience > Configuration.

Backend:

Integration base class: new settings_fields property, getter/setter methods with type-aware sanitization, register_settings_fields for declaring settings fields, and get_settings_config() for API responses.
Integrations: new get_all_integration_settings() and update_integration_settings() static methods.
Audience_Integrations wizard: REST API endpoints (GET /settings, POST /settings) gated behind a NEWSPACK_INTEGRATIONS_SETTINGS_ENABLED feature flag.

Frontend:

React settings page under the Audience wizard that renders each integration's settings fields and supports save/reset.
Page is conditionally registered only when the feature flag is enabled

How to test the changes in this Pull Request:

Before testing, make sure either Automattic/newspack-newsletters#2028 has been merged, or you checkout the branch.

Enable the feature by adding the NEWSPACK_INTEGRATIONS_SETTINGS_ENABLED flag to wp-config.php. Also enabled logging to more easily identify sync data in the logs (define( 'Newspack_Log_Level', 3 );)
Build the plugin: n build newspack-plugin.
Navigate to Audience > Integrations in wp-admin.
Verify that integrations with declared settings_fields appear with their fields rendered correctly.
Change values, save, and confirm persistence on page reload.
As a reader create a new reader account and verify the sync looks correct.
Disable the feature flag in wp-config.php and confirm the page no longer appears.
As a reader create a new reader account and verify the sync looks correct. (Should follow settings defined in Audience > Configuration).

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

includes/reader-activation/integrations/class-contact-pull.php

chickenn00dle · 2026-03-09T17:39:16Z

includes/reader-activation/integrations/class-esp.php

+	 */
+	public function register_settings_fields() {
+		$fields = [];
+		if ( ! Audience_Integrations::is_enabled() || ! Reader_Activation::is_esp_configured() ) {


One thing I wasn't certain about is whether we wanted to move the main esp configuration (api keys) into the integration. I opted to only move sync settings here and leave the main configuration in newsletter settings since newsletters should be usable without sync.

Copilot

Pull request overview

This PR adds a settings framework for reader activation integrations. It allows integrations (starting with the ESP integration) to declare settings fields that are stored as WordPress options and managed via a new Audience Integrations admin page, gated behind a NEWSPACK_INTEGRATIONS_SETTINGS_ENABLED constant.

Changes:

New Audience_Integrations wizard class with REST API endpoints (GET /settings, POST /settings, POST /settings/{id}/enabled) for managing integration settings
Integration base class extended with settings_fields, register_settings_fields() abstract method, settings getters/setters, metadata prefix management, and a unified get_settings_config() API method
Renaming of OPTION_PREFIX → INCOMING_FIELDS_OPTION_PREFIX, get_selected_fields() → get_incoming_fields(), set_selected_fields() → set_incoming_fields()

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`includes/reader-activation/integrations/class-integration.php`	Core: adds settings fields framework, metadata prefix management, renames incoming fields constants/methods, merges `get_enabled_outgoing_fields_raw_keys()` and `get_enabled_outgoing_fields_prefixed_keys()` into unified `get_enabled_outgoing_fields_keys($prefixed)`
`includes/reader-activation/integrations/class-esp.php`	Implements `register_settings_fields()`, `get_provider()`, `get_list_options()`, `get_master_list_id()`; adds feature flag branching in `can_sync()`, `push_contact_data()`, `get_incoming_available_contact_fields()`
`includes/reader-activation/class-integrations.php`	Adds `get_all_integration_settings()` and `update_integration_settings()` static methods; changes ESP auto-enable logic
`includes/reader-activation/class-reader-activation.php`	Routes ESP settings through the integration when feature flag is on
`includes/reader-activation/sync/class-metadata.php`	Routes `get_prefix()` through ESP integration when flag is on; renames method references in deprecated methods
`includes/reader-activation/integrations/class-contact-pull.php`	Updates renamed method calls from `get_selected_fields()` to `get_incoming_fields()`
`includes/wizards/audience/class-audience-integrations.php`	New wizard file providing admin page and REST API endpoints
`src/wizards/audience/views/integrations/index.js`	New React UI for the Integrations settings page
`src/wizards/index.tsx`	Conditionally registers the new integrations page component
`src/wizards/types/window.d.ts`	TypeScript type declaration for `newspackAudienceIntegrations` global
`includes/class-newspack.php`	Includes the new wizard file
`includes/class-wizards.php`	Instantiates `Audience_Integrations` wizard
`tests/unit-tests/integrations/class-test-integrations.php`	Adds new tests for metadata prefix, settings config, and renames test method names
`tests/unit-tests/integrations/class-sample-integration.php`	Implements `register_settings_fields()` abstract method
`tests/unit-tests/integrations/class-failing-sample-integration.php`	Implements `register_settings_fields()` abstract method (with a bug)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

includes/reader-activation/integrations/class-integration.php

tests/unit-tests/integrations/class-failing-sample-integration.php

includes/reader-activation/integrations/class-integration.php

src/wizards/audience/views/integrations/index.js

includes/wizards/audience/class-audience-integrations.php

src/wizards/audience/views/integrations/index.js

includes/reader-activation/integrations/class-integration.php

chickenn00dle · 2026-03-09T19:45:15Z

I had a look at the design update from Thomas and had a go at re-implementing this, but decided it might be better to push out what I have already and update the UI in a follow-up PR.

leogermani · 2026-03-10T21:37:10Z

I had a look at the design update from Thomas and had a go at re-implementing this, but decided it might be better to push out what I have already and update the UI in a follow-up PR.

I had a first pass at the code... I think we are touching more things than we should be. The prefix was intentionally left as a global option in #4544... and all those metadata methods are super fragile

Let's connect tomorrow and look at this together

leogermani

I gave it a thorough review now, and this is looking good. I left some comments in the code.

One of the points is that I think we should avoid that check to see if the new UI is enabled or not in order to route the fetching and setting of the settings. We should do something similar we did here.

The ideas is that we deprecate these global settings, and then route them to the ESP integration. First time we try to fetch them, we migrate the values from the global settings to the ESP integration setting.

This is cool because then the two UIs can actually exist while we are still in development and the transition is seamless. People will already be tweaking the ESP integration settings from the OLD UI - This is already what happens with enabled_outgoing_fields.

At first I thought we could leave the prefix as a global option. But it wasn't such a strong opinion. Now that you've done it, I think it's fine.

The UI for the incoming fields should also be a list of checkboxes. The available values come from get_incoming_available_contact_fields (that could be renamed to get_available_incoming_contact_fields)

leogermani · 2026-03-11T14:54:15Z

includes/reader-activation/integrations/class-esp.php

+		$provider = $this->get_provider();
+
+		// For Mailchimp, filter out groups and tags, only include remote lists.
+		if ( 'mailchimp' === $provider ) {


Mailchimp has a handy method to do that:

function get_lists( $audiences_only = false ) {

Thanks! Updated in 05014d7

leogermani · 2026-03-11T14:56:21Z

includes/reader-activation/integrations/class-esp.php

-				'ras_esp_master_list_id_not_found',
-				__( 'ESP master list ID is not set.', 'newspack-plugin' )
-			);
+		if ( Audience_Integrations::is_enabled() ) {


We shouldn't have this check. We should automatically route everything to the integration. For now, this ESP integration will be enabled by default

Updated in bc1448a

leogermani · 2026-03-11T14:58:20Z

includes/reader-activation/integrations/class-esp.php

 		}

-		$master_list_id = Reader_Activation::get_esp_master_list_id();
+		if ( Audience_Integrations::is_enabled() ) {


Again we should not have this check. We should migrate the options to the integration the first time we try to fetch it. Like we did here:

https://github.com/Automattic/newspack-plugin/pull/4544/changes#diff-b7214d34a99a1e90de19391b14bc5da959a54f98dd690ae60b2cebb384a4b89fR45

Great call! Updated in bc1448a

leogermani · 2026-03-11T15:04:42Z

includes/reader-activation/integrations/class-integration.php

-	public function set_selected_fields( $fields ) {
-		return \update_option( self::OPTION_PREFIX . $this->id, array_values( $fields ) );
+	public function get_enabled_outgoing_fields() {
+		return array_values( \get_option( self::OUTGOING_FIELDS_OPTION_PREFIX . $this->id, Sync\Metadata::get_default_fields() ) );


The default currently is an empty array. I think that's better

Hmm, I haven't changed the default for this one. Here it is in trunk:

newspack-plugin/includes/reader-activation/integrations/class-integration.php

Lines 257 to 259 in 6284fc6

public function get_enabled_outgoing_fields() {

return array_values( \get_option( self::OUTGOING_FIELDS_OPTION_PREFIX . $this->id, Sync\Metadata::get_default_fields() ) );

}

But I went ahead and updated this to an empty array in 71e2e25

leogermani · 2026-03-11T15:05:46Z

includes/reader-activation/integrations/class-integration.php

 	 */
-	public function get_selected_fields() {
-		return \get_option( self::OPTION_PREFIX . $this->id, [] );
+	public function get_incoming_fields() {


should be get_enabled_incoming_fields?

I originally renamed this while looking at the POC which allowed users to enter any field values. But seeing this has changes, I've renamed all of the relevant methods to get_enabled_...

leogermani · 2026-03-11T15:06:20Z

includes/reader-activation/integrations/class-integration.php

 	 */
-	public function get_enabled_outgoing_fields() {
-		return array_values( \get_option( self::OUTGOING_FIELDS_OPTION_PREFIX . $this->id, Sync\Metadata::get_default_fields() ) );
+	public function update_incoming_fields( $fields ) {


should be update_enabled

Fixed in 05014d7

leogermani · 2026-03-11T15:11:23Z

includes/reader-activation/sync/class-metadata.php

 	 */
 	public static function get_prefix() {
+		// When the integrations feature is enabled, read prefix from the ESP integration.
+		if ( \Newspack\Audience_Integrations::is_enabled() ) {


remove this check and directly route it to the esp integration like we did in other methods in this class

Updated in bc1448a

leogermani · 2026-03-11T15:12:21Z

includes/reader-activation/class-integrations.php


-		// hardcode ESP integration as enabled for now.
-		self::enable( 'esp' );
+		// Auto-enable all integrations if the option has never been saved.


This was arbitrarily done by Claude and I never revisited. I reverted this to hardcode ESP in 5fa536a

leogermani · 2026-03-11T15:13:55Z

includes/reader-activation/class-reader-activation.php

 		}
+
+		// When integration settings are enabled, route ESP settings to the integration.
+		if ( Audience_Integrations::is_enabled() ) {


Let's avoid this check and simply route the settings that we moved to the esp integration directly

Done in bc1448a

chickenn00dle · 2026-03-11T21:13:15Z

Thanks for the review @leogermani! Just noting I've tackled most of the feedback, but ran into one small issue while making changes and I'm out of time today so will have to pick this back up tomorrow.

chickenn00dle · 2026-03-12T17:00:58Z

This one is ready for another look @leogermani

One other issue I ran into while accounting for fetching incoming fields is, when the admin changes the metadata prefix, the incoming fields get populated with fields which contain the old prefix. For example, TEST_Account here:

For the time being I decided to tackle this in a follow-up PR, but let me know if you prefer I do this here instead.

leogermani · 2026-03-12T17:29:17Z

This one is ready for another look @leogermani

One other issue I ran into while accounting for fetching incoming fields is, when the admin changes the metadata prefix, the incoming fields get populated with fields which contain the old prefix. For example, TEST_Account here:

For the time being I decided to tackle this in a follow-up PR, but let me know if you prefer I do this here instead.

Got it. I think this is a non-issue... part of the challenges of dealing with complex data flows. If someone ever changes the prefix, they might have to do some cleanup in the data in the integration

leogermani · 2026-03-12T18:05:29Z

includes/reader-activation/integrations/class-esp.php

+		}
+		$list_options = $this->get_list_options();
+		$provider     = $this->get_provider();
+		switch ( $provider->service ) {


$provider can be null. Wouldn't this throw an alert or error here?

Ah. Yes it would. Fixed in 07960ce

leogermani · 2026-03-12T18:12:35Z

includes/reader-activation/class-integrations.php

 		do_action( 'newspack_reader_activation_register_integrations' );

-		// hardcode ESP integration as enabled for now.
+		// hardcode ESP integration as enabled for now..


🤦 Fixed in 07960ce

leogermani · 2026-03-12T18:13:04Z

includes/reader-activation/sync/class-metadata.php

 	 * @return string
 	 */
 	public static function get_prefix() {
+		$esp_integration = Integrations::get_integration( 'esp' );


let's add a deprecation doc comment and an explanation at the doc block like we did to the other methods

Added in 07960ce

leogermani · 2026-03-12T18:17:59Z

includes/reader-activation/integrations/class-esp.php

 			return $fields;
 		}

+		if ( $filtered ) {


I this is not specific to the ESP integration. We should have this filter on the Integration abstraction, like it was... no?

So the integration implementation just grab all the fields, and our framework will automatically filter out our own fields from the list

I initially moved this thinking we could allow individual integrations to apply their own filtering logic for any special cases. But you're right that it makes more sense to live in the integration abstraction.

I moved it back in 07960ce

changes applied! Thanks Raz!

github-actions · 2026-03-12T20:46:37Z

Hey @chickenn00dle, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

This fixes a fatal that resulted in calling the incorrect is_enabled method in #4541

chickenn00dle force-pushed the feat/add-integrations-settings branch from a2bf584 to 76aa8b3 Compare March 4, 2026 17:08

chickenn00dle commented Mar 4, 2026

View reviewed changes

includes/reader-activation/integrations/class-contact-pull.php Outdated Show resolved Hide resolved

chickenn00dle force-pushed the feat/add-integrations-settings branch from 39a7855 to 04f1105 Compare March 9, 2026 16:30

chickenn00dle changed the title ~~feat: add integrations settings~~ feat(integrations): add integrations settings Mar 9, 2026

chickenn00dle commented Mar 9, 2026

View reviewed changes

chickenn00dle marked this pull request as ready for review March 9, 2026 18:06

chickenn00dle requested a review from a team as a code owner March 9, 2026 18:06

Copilot AI review requested due to automatic review settings March 9, 2026 18:06

Copilot started reviewing on behalf of chickenn00dle March 9, 2026 18:06 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Mar 9, 2026

leogermani previously requested changes Mar 11, 2026

View reviewed changes

chickenn00dle force-pushed the feat/add-integrations-settings branch from 5d22ba4 to 8a6fb10 Compare March 11, 2026 21:00

chickenn00dle force-pushed the feat/add-integrations-settings branch from 99af4ba to 11cdebd Compare March 11, 2026 21:14

chickenn00dle added 13 commits March 12, 2026 10:21

feat: add integrations settings

536eb88

feat: move sync settings

bc0ef41

feat: add custom meta field

4abb745

fix: tests

e59a500

fix: register metadata fields for every integration

ec19481

fix: utilize metadata refactor

6efb3cd

fix: update incoming getter/setter methods

c6def1d

fix: integration level metadata prefix

0d30f53

fix: add unit tests

49b813b

fix: use esp integration settings

b4fb102

fix: copilot suggestions

869ce0f

fix: phpunit

2a31591

fix: feedback round 1 changes

7a43210

chickenn00dle added 4 commits March 12, 2026 10:21

fix: lazy migrate settings

3f9d487

fix: incoming metadata settings

5fa536a

fix: unit tests

87437d1

fix: infinite loop

61341a8

chickenn00dle force-pushed the feat/add-integrations-settings branch from 11cdebd to 61341a8 Compare March 12, 2026 14:21

chickenn00dle added 2 commits March 12, 2026 11:04

fix: unecessary rerenders

99c42a9

fix: filtered incoming fields

da209f2

chickenn00dle force-pushed the feat/add-integrations-settings branch 2 times, most recently from 5e0149c to 0faff0a Compare March 12, 2026 16:50

fix: phpunit tests

71e2e25

chickenn00dle force-pushed the feat/add-integrations-settings branch from 0faff0a to 71e2e25 Compare March 12, 2026 16:53

chickenn00dle requested a review from leogermani March 12, 2026 17:26

leogermani reviewed Mar 12, 2026

View reviewed changes

fix: feedback round 2

07960ce

chickenn00dle requested a review from leogermani March 12, 2026 19:46

leogermani approved these changes Mar 12, 2026

View reviewed changes

github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Mar 12, 2026

chickenn00dle merged commit 735e640 into trunk Mar 12, 2026
17 checks passed

chickenn00dle deleted the feat/add-integrations-settings branch March 12, 2026 20:46

github-actions bot added the needs-changelog label Mar 12, 2026

chickenn00dle removed the needs-changelog label Mar 12, 2026

chickenn00dle mentioned this pull request Mar 13, 2026

fix: use correct is_enabled method #4564

Merged

6 tasks

chickenn00dle added a commit that referenced this pull request Mar 13, 2026

fix: use correct is_enabled method (#4564)

6396e41

This fixes a fatal that resulted in calling the incorrect is_enabled method in #4541

	public function get_enabled_outgoing_fields() {
	return array_values( \get_option( self::OUTGOING_FIELDS_OPTION_PREFIX . $this->id, Sync\Metadata::get_default_fields() ) );
	}

Conversation

chickenn00dle commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

Uh oh!

chickenn00dle Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chickenn00dle commented Mar 9, 2026

Uh oh!

leogermani commented Mar 10, 2026

Uh oh!

leogermani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenn00dle Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenn00dle Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenn00dle commented Mar 11, 2026

Uh oh!

chickenn00dle commented Mar 12, 2026

Uh oh!

leogermani commented Mar 12, 2026

chickenn00dle commented Mar 4, 2026 •

edited

Loading

chickenn00dle Mar 9, 2026 •

edited

Loading

chickenn00dle Mar 12, 2026 •

edited

Loading

chickenn00dle Mar 11, 2026 •

edited

Loading